Conversation
There was a problem hiding this comment.
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
Files not reviewed (1)
- samples/CommunityToolkit.Maui.Sample/Pages/Alerts/SnackbarPage.xaml: Language not supported
Comments suppressed due to low confidence (2)
src/CommunityToolkit.Maui.Core/Views/Alert/AlertView.macios.cs:60
- Ensure that the new layout behavior introduced in LayoutSubviews is covered by tests.
public override void LayoutSubviews()
samples/CommunityToolkit.Maui.Sample/Pages/Alerts/SnackbarPage.xaml.cs:85
- [nitpick] The method name 'DisplayCustomSnackbarButtonClicked2' is ambiguous and should be renamed to something more descriptive, such as 'DisplayCustomSnackbarWithOptions'.
async void DisplayCustomSnackbarButtonClicked2(object sender, EventArgs e)
VladislavAntonyuk
force-pushed
the
1901-fix-snackbar
branch
from
3000b06 to
ee5b86c
Compare
VladislavAntonyuk
force-pushed
the
1901-fix-snackbar
branch
from
3ddb104 to
16919f7
Compare
`DisplayCustomSnackbarButtonWithAndchor` is used to demonstrate setting an Anchor on a Snackbar
…e, Update Displayed text
TheCodeTraveler
left a comment
TheCodeTraveler
left a comment
There was a problem hiding this comment.
Thanks Vlad - this looks great! 💯
Just one problem we should fix before merging: The app crashes when a Popup with an anchor is displayed and while it is currently displayed we navigate away from the page.
Here's two examples demonstrating this using the Sample App:
samples/CommunityToolkit.Maui.Sample/Pages/Alerts/SnackbarPage.xaml.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Copilot reviewed 5 out of 7 changed files in this pull request and generated no comments.
Files not reviewed (2)
- samples/CommunityToolkit.Maui.Sample/Pages/Alerts/SnackbarPage.xaml: Language not supported
- src/CommunityToolkit.Maui.Core/Views/Toast/PlatformToast.macios.cs: Evaluated as low risk
ne0rrmatrix
left a comment
ne0rrmatrix
left a comment
There was a problem hiding this comment.
Looks very well done! Just need to fix that IOS bug and it will be good to merge. The issue was listed previously by the @TheCodeTraveler. Once that is cleared i will be happy to approve this. The look and feel is now consistant across both android and ios.
ne0rrmatrix
left a comment
ne0rrmatrix
left a comment
There was a problem hiding this comment.
Changes look. Good I have tested them on IOS and the crash is no longer an issue. Only one minor bug that might be nice to fix before merge.
2025-01-28.09-37-05.mp4
VladislavAntonyuk
added
the
needs discussion
3 participants
Description of Change
Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information